Custom logger for Swarming & Removes circular dependency at logs.py#5247
Custom logger for Swarming & Removes circular dependency at logs.py#5247
Conversation
|
|
||
| def is_tworker(): | ||
| return get_value('TWORKER', False) | ||
|
|
There was a problem hiding this comment.
Note for reviews:
To remove a circular dependency between logs.py and environment.py we moved this method into run_bot.py, this method required the logs module hence causing a circular dependency. Its safe to remove it from here as this was really only used at run_bot.py
| return stacktrace, clean_exit, task_payload | ||
|
|
||
|
|
||
| def update_task_enabled() -> bool: |
There was a problem hiding this comment.
See context at context for this change at this comment and this other comment
There was a problem hiding this comment.
We should mention this change in the PR description (which will become the commit message).
letitz
left a comment
There was a problem hiding this comment.
Currently swarming bots use the K8s default logging configurations(which work fine :D), but when we try to find logs specific to a given swarming bot host or a swarming task, things start to get complicated, its hard to pinpoint the execution workflow.
In this PR we changed the configuration so the logger can displays different information if its running on swarming.
This is vague. Please mention what is actually changing:
- new metadata fields on logs
- dropped metadata fields
- any other behavioral changes from the PR
- any refactorings done along the way
LGTM otherwise to avoid blocking on me further, but please resolve comments before landing.
| not _is_running_on_app_engine() and not _is_running_on_k8s()) | ||
| This is enabled by default but disabled for local development.""" | ||
| return environment.get_value('LOG_TO_GCP', | ||
| True) and (not os.getenv('PY_UNITTESTS') and |
There was a problem hiding this comment.
Optional: I would move this PY_UNITTESTS check to its own helper method in environment.py: is_running_unit_tests().
| return stacktrace, clean_exit, task_payload | ||
|
|
||
|
|
||
| def update_task_enabled() -> bool: |
There was a problem hiding this comment.
We should mention this change in the PR description (which will become the commit message).
Sure i'll do that, thanks for the feedback :D |
Co-authored-by: Titouan Rigoudy <titouan@chromium.org>
Currently swarming bots use the K8s default logging configurations(which work fine :D), but when we try to find logs specific to a given swarming bot host or a swarming task, things start to get complicated, its hard to pinpoint the execution workflow.
In this PR we changed the configuration so the logger can displays different information if its running on swarming.
Note: This PR is complimentary to this other pr
Tests performed
I launched some fuzzing jobs that executed in swarming, thanks to the new fields i was able to easily find the logs for a given bot or a given task. See example